-
Notifications
You must be signed in to change notification settings - Fork 64
Add TaskValidation module
#313
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
tests/FsToolkit.ErrorHandling.Tests/FsToolkit.ErrorHandling.Tests.fsproj
Outdated
Show resolved
Hide resolved
tw0po1nt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self-review complete
TheAngryByrd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lots of great work here!
I left some comments on areas I think are missing.
| Expect.equal actual (Validation.error errorMsg) "Should be Error" | ||
| } | ||
|
|
||
| testCaseTask "Fail Path Validation/Choice/AsyncValidation" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless I don't see it, I don't think there's tests covering seeing multiple failure cases. Something like how
| testCase "Fail Path Result" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a test covering that scenario in latest commit. Let me know if that is what you were looking for.
| |> Async.StartImmediateAsTask | ||
|
|
||
| [<AutoOpen>] | ||
| module TaskValidationCEExtensionsHighPriority2 = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is missing overloads for Result<'a,'err> (where 'err is not a list). You can see examples of the overload resolution in AsyncValidation:
| module MediumPriority = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added these overloads
TheAngryByrd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️ Thank you!
- BREAKING: [Remove Ply and update to FSharp 6](#248) Credits @TheAngryByrd - BREAKING: [Remove MergeSources (and!) from some implementations like Result](#261) Credits @TheAngryByrd - BREAKING: [Merge TaskResult into Core library](#285) Credits @TheAngryByrd - This means FsToolkit.ErrorHandling.TaskResult is no longer a separate package and will not be updated. It is now part of the core library. - BREAKING: [Rename retn to singleton](#287) Credits @1eyewonder - BREAKING: [Rename returnError to error + documentation](#311) Credits @tw0po1nt - [Use Microsoft.Bcl.AsyncInterfaces in netstandard2.0 (Allows IAsyncDisposable and IAsyncEnumerable)](#250) Credits @TheAngryByrd - [Build against Net8](#251) Credits @TheAngryByrd - [Fix Overload Resolution to Align to Computation Expression used](#252) Credits @TheAngryByrd - [refactor!: Seq.sequenceResultM returns Array instead of seq](#255) Credits @bartelink - [feat(Seq): sequenceResultA](#255) Credits @bartelink - [Updated uses of Seq.append](#290) Credits @1eyewonder - [Add Option.traverseAsync and Option.sequenceAsync](#298 (comment)) Credits @tw0po1nt - [Add Require and Check helper methods](#295) Credits @PI-Gorbo - [Add new AsyncOption APIs and document all its other functions; minor fixes to documentation for Option module](#307) Credits @tw0po1nt - [F# 9 support and nullness](#308) Credits @TheAngryByrd - [Update IcedTasks 0.11.7](0a4cc7b) Credits @TheAngryByrd - [Add TaskValidation module](#313) Credits @tw0po1n8
- BREAKING: [Remove Ply and update to FSharp 6](#248) Credits @TheAngryByrd - BREAKING: [Remove MergeSources (and!) from some implementations like Result](#261) Credits @TheAngryByrd - BREAKING: [Merge TaskResult into Core library](#285) Credits @TheAngryByrd - This means FsToolkit.ErrorHandling.TaskResult is no longer a separate package and will not be updated. It is now part of the core library. - BREAKING: [Rename retn to singleton](#287) Credits @1eyewonder - BREAKING: [Rename returnError to error + documentation](#311) Credits @tw0po1nt - [Use Microsoft.Bcl.AsyncInterfaces in netstandard2.0 (Allows IAsyncDisposable and IAsyncEnumerable)](#250) Credits @TheAngryByrd - [Build against Net8](#251) Credits @TheAngryByrd - [Fix Overload Resolution to Align to Computation Expression used](#252) Credits @TheAngryByrd - [refactor!: Seq.sequenceResultM returns Array instead of seq](#255) Credits @bartelink - [feat(Seq): sequenceResultA](#255) Credits @bartelink - [Updated uses of Seq.append](#290) Credits @1eyewonder - [Add Option.traverseAsync and Option.sequenceAsync](#298 (comment)) Credits @tw0po1nt - [Add Require and Check helper methods](#295) Credits @PI-Gorbo - [Add new AsyncOption APIs and document all its other functions; minor fixes to documentation for Option module](#307) Credits @tw0po1nt - [F# 9 support and nullness](#308) Credits @TheAngryByrd - [Update IcedTasks 0.11.7](0a4cc7b) Credits @TheAngryByrd - [Add TaskValidation module](#313) Credits @tw0po1nt - [feat(Seq.traverse/sequence*)!: Yield arrays](#310) Credits @bartelink
- BREAKING: [Remove Ply and update to FSharp 6](#248) Credits @TheAngryByrd - BREAKING: [Remove MergeSources (and!) from some implementations like Result](#261) Credits @TheAngryByrd - BREAKING: [Merge TaskResult into Core library](#285) Credits @TheAngryByrd - This means FsToolkit.ErrorHandling.TaskResult is no longer a separate package and will not be updated. It is now part of the core library. - BREAKING: [Rename retn to singleton](#287) Credits @1eyewonder - BREAKING: [Rename returnError to error + documentation](#311) Credits @tw0po1nt - [Use Microsoft.Bcl.AsyncInterfaces in netstandard2.0 (Allows IAsyncDisposable and IAsyncEnumerable)](#250) Credits @TheAngryByrd - [Build against Net8](#251) Credits @TheAngryByrd - [Fix Overload Resolution to Align to Computation Expression used](#252) Credits @TheAngryByrd - [refactor!: Seq.sequenceResultM returns Array instead of seq](#255) Credits @bartelink - [feat(Seq): sequenceResultA](#255) Credits @bartelink - [Updated uses of Seq.append](#290) Credits @1eyewonder - [Add Option.traverseAsync and Option.sequenceAsync](#298 (comment)) Credits @tw0po1nt - [Add Require and Check helper methods](#295) Credits @PI-Gorbo - [Add new AsyncOption APIs and document all its other functions; minor fixes to documentation for Option module](#307) Credits @tw0po1nt - [F# 9 support and nullness](#308) Credits @TheAngryByrd - [Update IcedTasks 0.11.7](0a4cc7b) Credits @TheAngryByrd - [Add TaskValidation module](#313) Credits @tw0po1nt - [feat(Seq.traverse/sequence*)!: Yield arrays](#310) Credits @bartelink - [Add ParallelAsync CEs](#318) Credits @njlr
- BREAKING: [Remove Ply and update to FSharp 6](#248) Credits @TheAngryByrd - BREAKING: [Remove MergeSources (and!) from some implementations like Result](#261) Credits @TheAngryByrd - BREAKING: [Merge TaskResult into Core library](#285) Credits @TheAngryByrd - This means FsToolkit.ErrorHandling.TaskResult is no longer a separate package and will not be updated. It is now part of the core library. - BREAKING: [Rename retn to singleton](#287) Credits @1eyewonder - BREAKING: [Rename returnError to error + documentation](#311) Credits @tw0po1nt - [Use Microsoft.Bcl.AsyncInterfaces in netstandard2.0 (Allows IAsyncDisposable and IAsyncEnumerable)](#250) Credits @TheAngryByrd - [Build against Net8](#251) Credits @TheAngryByrd - [Fix Overload Resolution to Align to Computation Expression used](#252) Credits @TheAngryByrd - [refactor!: Seq.sequenceResultM returns Array instead of seq](#255) Credits @bartelink - [feat(Seq): sequenceResultA](#255) Credits @bartelink - [Updated uses of Seq.append](#290) Credits @1eyewonder - [Add Option.traverseAsync and Option.sequenceAsync](#298 (comment)) Credits @tw0po1nt - [Add Require and Check helper methods](#295) Credits @PI-Gorbo - [Add new AsyncOption APIs and document all its other functions; minor fixes to documentation for Option module](#307) Credits @tw0po1nt - [F# 9 support and nullness](#308) Credits @TheAngryByrd - [Update IcedTasks 0.11.7](0a4cc7b) Credits @TheAngryByrd - [Add TaskValidation module](#313) Credits @tw0po1nt - [feat(Seq.traverse/sequence*)!: Yield arrays](#310) Credits @bartelink - [Add ParallelAsync CEs](#318) Credits @njlr - [Add Option.sequenceAsyncResult and Option.traverseAsyncResult](#321) Credits @JayWearsSocks
- BREAKING: [Remove Ply and update to FSharp 6](#248) Credits @TheAngryByrd - BREAKING: [Remove MergeSources (and!) from some implementations like Result](#261) Credits @TheAngryByrd - BREAKING: [Merge TaskResult into Core library](#285) Credits @TheAngryByrd - This means FsToolkit.ErrorHandling.TaskResult is no longer a separate package and will not be updated. It is now part of the core library. - BREAKING: [Rename retn to singleton](#287) Credits @1eyewonder - BREAKING: [Rename returnError to error + documentation](#311) Credits @tw0po1nt - [Use Microsoft.Bcl.AsyncInterfaces in netstandard2.0 (Allows IAsyncDisposable and IAsyncEnumerable)](#250) Credits @TheAngryByrd - [Build against Net8](#251) Credits @TheAngryByrd - [Fix Overload Resolution to Align to Computation Expression used](#252) Credits @TheAngryByrd - [refactor!: Seq.sequenceResultM returns Array instead of seq](#255) Credits @bartelink - [feat(Seq): sequenceResultA](#255) Credits @bartelink - [Updated uses of Seq.append](#290) Credits @1eyewonder - [Add Option.traverseAsync and Option.sequenceAsync](#298 (comment)) Credits @tw0po1nt - [Add Require and Check helper methods](#295) Credits @PI-Gorbo - [Add new AsyncOption APIs and document all its other functions; minor fixes to documentation for Option module](#307) Credits @tw0po1nt - [F# 9 support and nullness](#308) Credits @TheAngryByrd - [Update IcedTasks 0.11.7](0a4cc7b) Credits @TheAngryByrd - [Add TaskValidation module](#313) Credits @tw0po1nt - [feat(Seq.traverse/sequence*)!: Yield arrays](#310) Credits @bartelink - [Add ParallelAsync CEs](#318) Credits @njlr - [Add Option.sequenceAsyncResult and Option.traverseAsyncResult](#321) Credits @JayWearsSocks - [Add traversals/sequences for Task and TaskResult in the Option module](#325) Credits @tw0po1nt
- BREAKING: [Remove Ply and update to FSharp 6](#248) Credits @TheAngryByrd - BREAKING: [Remove MergeSources (and!) from some implementations like Result](#261) Credits @TheAngryByrd - BREAKING: [Merge TaskResult into Core library](#285) Credits @TheAngryByrd - This means FsToolkit.ErrorHandling.TaskResult is no longer a separate package and will not be updated. It is now part of the core library. - BREAKING: [Rename retn to singleton](#287) Credits @1eyewonder - BREAKING: [Rename returnError to error + documentation](#311) Credits @tw0po1nt - [Use Microsoft.Bcl.AsyncInterfaces in netstandard2.0 (Allows IAsyncDisposable and IAsyncEnumerable)](#250) Credits @TheAngryByrd - [Build against Net8](#251) Credits @TheAngryByrd - [Fix Overload Resolution to Align to Computation Expression used](#252) Credits @TheAngryByrd - [refactor!: Seq.sequenceResultM returns Array instead of seq](#255) Credits @bartelink - [feat(Seq): sequenceResultA](#255) Credits @bartelink - [Updated uses of Seq.append](#290) Credits @1eyewonder - [Add Option.traverseAsync and Option.sequenceAsync](#298 (comment)) Credits @tw0po1nt - [Add Require and Check helper methods](#295) Credits @PI-Gorbo - [Add new AsyncOption APIs and document all its other functions; minor fixes to documentation for Option module](#307) Credits @tw0po1nt - [F# 9 support and nullness](#308) Credits @TheAngryByrd - [Update IcedTasks 0.11.7](0a4cc7b) Credits @TheAngryByrd - [Add TaskValidation module](#313) Credits @tw0po1nt - [feat(Seq.traverse/sequence*)!: Yield arrays](#310) Credits @bartelink - [Add ParallelAsync CEs](#318) Credits @njlr - [Add Option.sequenceAsyncResult and Option.traverseAsyncResult](#321) Credits @JayWearsSocks - [Add traversals/sequences for Task and TaskResult in the Option module](#325) Credits @tw0po1nt - [Add ok and error helper functions to TaskResultOption and AsyncResultOption modules](#327) Credits @tw0po1nt - [Add CancellableTaskOption module and CE + tests and documentation](#328) Credits @tw0po1nt
- BREAKING: [Remove Ply and update to FSharp 6](#248) Credits @TheAngryByrd - BREAKING: [Remove MergeSources (and!) from some implementations like Result](#261) Credits @TheAngryByrd - BREAKING: [Merge TaskResult into Core library](#285) Credits @TheAngryByrd - This means FsToolkit.ErrorHandling.TaskResult is no longer a separate package and will not be updated. It is now part of the core library. - BREAKING: [Rename retn to singleton](#287) Credits @1eyewonder - BREAKING: [Rename returnError to error + documentation](#311) Credits @tw0po1nt - [Use Microsoft.Bcl.AsyncInterfaces in netstandard2.0 (Allows IAsyncDisposable and IAsyncEnumerable)](#250) Credits @TheAngryByrd - [Build against Net8](#251) Credits @TheAngryByrd - [Fix Overload Resolution to Align to Computation Expression used](#252) Credits @TheAngryByrd - [refactor!: Seq.sequenceResultM returns Array instead of seq](#255) Credits @bartelink - [feat(Seq): sequenceResultA](#255) Credits @bartelink - [Updated uses of Seq.append](#290) Credits @1eyewonder - [Add Option.traverseAsync and Option.sequenceAsync](#298 (comment)) Credits @tw0po1nt - [Add Require and Check helper methods](#295) Credits @PI-Gorbo - [Add new AsyncOption APIs and document all its other functions; minor fixes to documentation for Option module](#307) Credits @tw0po1nt - [F# 9 support and nullness](#308) Credits @TheAngryByrd - [Update IcedTasks 0.11.7](0a4cc7b) Credits @TheAngryByrd - [Add TaskValidation module](#313) Credits @tw0po1nt - [feat(Seq.traverse/sequence*)!: Yield arrays](#310) Credits @bartelink - [Add ParallelAsync CEs](#318) Credits @njlr - [Add Option.sequenceAsyncResult and Option.traverseAsyncResult](#321) Credits @JayWearsSocks - [Add traversals/sequences for Task and TaskResult in the Option module](#325) Credits @tw0po1nt - [Add ok and error helper functions to TaskResultOption and AsyncResultOption modules](#327) Credits @tw0po1nt - [Add CancellableTaskOption module and CE + tests and documentation](#328) Credits @tw0po1nt - [Remove paket, Enforce nullness on net9.0, remove mocha](#331) Credits @TheAngryByrd - [Add TaskValueOption module, operators, and CE + tests and documentation](#329) Credits @tw0po1nt
- BREAKING: [Remove Ply and update to FSharp 6](#248) Credits @TheAngryByrd - BREAKING: [Remove MergeSources (and!) from some implementations like Result](#261) Credits @TheAngryByrd - BREAKING: [Merge TaskResult into Core library](#285) Credits @TheAngryByrd - This means FsToolkit.ErrorHandling.TaskResult is no longer a separate package and will not be updated. It is now part of the core library. - BREAKING: [Rename retn to singleton](#287) Credits @1eyewonder - BREAKING: [Rename returnError to error + documentation](#311) Credits @tw0po1nt - [Use Microsoft.Bcl.AsyncInterfaces in netstandard2.0 (Allows IAsyncDisposable and IAsyncEnumerable)](#250) Credits @TheAngryByrd - [Build against Net8](#251) Credits @TheAngryByrd - [Fix Overload Resolution to Align to Computation Expression used](#252) Credits @TheAngryByrd - [refactor!: Seq.sequenceResultM returns Array instead of seq](#255) Credits @bartelink - [feat(Seq): sequenceResultA](#255) Credits @bartelink - [Updated uses of Seq.append](#290) Credits @1eyewonder - [Add Option.traverseAsync and Option.sequenceAsync](#298 (comment)) Credits @tw0po1nt - [Add Require and Check helper methods](#295) Credits @PI-Gorbo - [Add new AsyncOption APIs and document all its other functions; minor fixes to documentation for Option module](#307) Credits @tw0po1nt - [F# 9 support and nullness](#308) Credits @TheAngryByrd - [Update IcedTasks 0.11.7](0a4cc7b) Credits @TheAngryByrd - [Add TaskValidation module](#313) Credits @tw0po1nt - [feat(Seq.traverse/sequence*)!: Yield arrays](#310) Credits @bartelink - [Add ParallelAsync CEs](#318) Credits @njlr - [Add Option.sequenceAsyncResult and Option.traverseAsyncResult](#321) Credits @JayWearsSocks - [Add traversals/sequences for Task and TaskResult in the Option module](#325) Credits @tw0po1nt - [Add ok and error helper functions to TaskResultOption and AsyncResultOption modules](#327) Credits @tw0po1nt - [Add CancellableTaskOption module and CE + tests and documentation](#328) Credits @tw0po1nt - [Remove paket, Enforce nullness on net9.0, remove mocha](#331) Credits @TheAngryByrd - [Add TaskValueOption module, operators, and CE + tests and documentation](#329) Credits @tw0po1nt
Proposed Changes
Add
TaskValidationmodule as proposed in #228Types of changes
What types of changes does your code introduce to FsToolkit.ErrorHandling?
Checklist
Further comments
This will probably warrant a decent amount of scrutiny. The
TaskValidationmodule mirrors theAsyncValidationmodule, along with its suite of tests, same with the ops. The code for the computation expression and its tests are derived from theTaskResultcomputation expression and its tests, since theAsyncValidationcomputation expression does not use the state machine. This should probably be fine, sinceValidationis simply aResultthat accumulates the errors. But it may warrant some additional,Validationspecific tests that I might have overlooked.